Easier specification of external file dependencies #83

ramnathv · 2015-02-18T18:45:38Z

This PR makes it easy for package developers and users to specify file dependencies for their widgets. Here is an example:

# install_github('htmlwidgets/datamaps')
library(datamaps)
datamaps(
  scope = 'pcs',
  geographyConfig = list(dataUrl = htmlwidgets:::attachment("data/pcs.json"))
)

It uses a similar mechanism to the one used by the JS function to mark objects as javascript literals. The resolution of attachment urls is carried out using the HTMLWidgets.getAttachmentUrl function.

@jcheng5 can you take a look at this whenever you have time. @jjallaire @timelyportfolio @yihui if you can test this functionality with any of your widgets and provide me with feedback/comments, that would be very useful.

jcheng5 · 2015-02-18T18:51:03Z

R/fileDependency.R

+#' Mark a string as an attachment
+#' @export
+attachment <- function(x){
+  structure(normalizePath(x), class = unique(c("ATTACHMENT", oldClass(x))))


Why oldClass vs. class here?

I just used the same code as used in htmlwidgets:::JS. I should admit I did not understand why oldClass was used there in the first place 😄

I should also probably add a file.exists(x) check in the attachment function to ensure that incorrect paths are caught early on.

yihui · 2015-02-19T15:03:39Z

R/fileDependency.R

+#' @export
+attachment <- function(x){
+  if (!file.exists(x)){
+    stop("The attachment ", x, " does not exist")


How about just leaving it to normalizePath(mustWork = TRUE)?

Good suggestion! I will make the change.

nsh87 · 2015-05-28T05:45:11Z

This would be a most-welcome feature. I have been looking at this issue in my attempt to attach a file dependency that gets created in one of my functions before htmlwidgets::createWidget. Thus far I have not been successful in making the file accessible through the browser.

jcheng5 · 2015-05-28T06:40:15Z

@nsh87 Does this data need to be packaged as a separate file? Could it perhaps be included as part of the createWidget's x value?

nsh87 · 2015-05-28T06:47:54Z

@jcheng5 It's an XML file, so I don't think it can be included with x since that goes through toJSON. Outside of htmlwidgets, the JS library I'm using reads the data from an XML file, typically through AJAX. What I'm attempting to do is to get the file into <head> as an attachment and try to read it from there, but no luck attaching it so far.

radial_phylo <- function(user-data, width=NULL, height=NULL) {

   # Do some stuff to save an XML file to xmlFilePath

   phyloxml <- htmltools::htmlDependency(
     name = 'phyloxmls',
     version = '1.0',
     src = c(file=xmlFilePath)
     attachment = list(xml = xmlFilePath)  # Pretty sure this isn't right
   )

  # create widget
  htmlwidgets::createWidget(
    name = 'radial_phylo',
    x,
    width = width,
    height = height,
    htmlwidgets::sizingPolicy(
      viewer.padding = 0,
      viewer.suppress = suppressViewer,
      browser.fill = TRUE
    ),
    package = 'receptormarker',
    dependencies = phyloxml
  )
}

jcheng5 · 2015-05-28T06:53:22Z

I think that's pretty close--in the JavaScript side you should then be able to call HTMLWidgets.getAttachmentUrl("phyloxmls", "xml") and get back a URL...?

nsh87 · 2015-05-28T06:57:56Z

That JS call works, but I'm not sure if the reference is right. If I inspect the attachment in the HTML and click on the link it's just a blank XML file (and xmlFilePath is not blank).

var phyloxmlfile = HTMLWidgets.getAttachmentUrl('phyloxmls', 'xml');
// phyloxmlfile = "lib/phyloxml-1.0//Users/Nikhil/dev/receptormarker/receptormarker/phyloxml-69647efd9cfd.xml"

nsh87 · 2015-05-28T07:02:37Z

Plus I don't have a "inst/htmlwidgets/lib/phyloxml-1.0" folder. Judging by the fact that it's looking in "lib/phyloxml-1.0", it looks like it's expecting the XML asset to exist in my package, but the XML file is user-generated.

jcheng5 · 2015-05-28T07:09:17Z

This chunk:

   phyloxml <- htmltools::htmlDependency(
     name = 'phyloxmls',
     version = '1.0',
     src = c(file=xmlFilePath)
     attachment = list(xml = xmlFilePath)  # Pretty sure this isn't right
   )

Should be this:

   phyloxml <- htmltools::htmlDependency(
     name = 'phyloxmls',
     version = '1.0',
     src = c(file=dirname(xmlFilePath))
     attachment = list(xml = basename(xmlFilePath))  # Pretty sure this isn't right
   )

Note that the entire contents of dirname(xmlFilePath) will be copied along with xmlFilePath, so if that includes a lot of extraneous stuff, copy your XML file to an empty temp directory and then pass that temp directory as your src (and still the basename() as the attachment value).

nsh87 · 2015-05-28T07:22:53Z

Awesome, that did the trick. Lines 7 and 8 in "R/fileDependency.R" at the top of this pull request should've been good hints for me. Thanks.

jcheng5 · 2015-05-28T08:48:16Z

OK, great, glad to hear it. :)

ramnathv · 2015-05-28T15:17:17Z

@jcheng5 Should we revisit this PR (i need some cleanup so it is mergeable)? All this PR does is provide a wrapper around the dependency API that you have put in. So maybe one way to go about this is to merge this in, acknowledging the limitations you have pointed out, and then once we have a more complete way of doing this, update the mechanism. Let me know what you think.

timelyportfolio · 2015-05-28T16:49:16Z

👍 +1

jcheng5 · 2015-05-28T19:31:34Z

I think it might be a simple tweak to get it working for dynamic values. Give me a minute...

ramnathv · 2015-05-28T19:33:44Z

@jcheng5 I look forward to your magic 👍

Without this change, the attachment() feature might return stale data, since it identified file dependencies only by basename.

jcheng5 · 2015-05-28T20:45:22Z

OK, I pushed some changes. Some notes:

I took dependencies on digest and bitops, hope that's OK.
Should fileDependency be exported? I'm not sure how one would use it correctly outside of attachment() (although perhaps that's only the case after my change).
The underlying mechanism for traversing the data object based on key, will only work if neither the attachments nor their parent lists are ever unnamed lists (nor may their names have periods in them).
I didn't write docs/examples, that would need to be done too before merging.

ramnathv · 2015-05-28T20:49:27Z

Thanks @jcheng5. I will play with your PR and do some testing. I am okay with the dependencies on digest and bitops. I will be happy to write up docs/examples before we merge.

ramnathv added 4 commits January 15, 2015 16:33

initial work on easier file dependencies.

5b912ab

resolve attachment urls automagically!

94fb57e

add attachments and dependencies for automatic resolution.

fbad626

add utility functions to support specification of file dependencies.

5f6348b

jcheng5 reviewed Feb 18, 2015
View reviewed changes

check if the file being attached exists

ed1ba29

yihui reviewed Feb 19, 2015
View reviewed changes

some minor changes (thanks @yihui)

cd9478c

Enhance attachment() feature to work with dynamic data

04fe1af

Without this change, the attachment() feature might return stale data, since it identified file dependencies only by basename.

hafen mentioned this pull request May 28, 2015

Robust javascript dependency handling in the browser #121

Closed

ramnathv mentioned this pull request Aug 22, 2015

Making rgl widget #141

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Easier specification of external file dependencies #83

Easier specification of external file dependencies #83

ramnathv commented Feb 18, 2015

jcheng5 Feb 18, 2015

ramnathv Feb 18, 2015

ramnathv Feb 18, 2015

yihui Feb 19, 2015

ramnathv Feb 19, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015

timelyportfolio commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015

Easier specification of external file dependencies #83

Are you sure you want to change the base?

Easier specification of external file dependencies #83

Conversation

ramnathv commented Feb 18, 2015

jcheng5 Feb 18, 2015

Choose a reason for hiding this comment

ramnathv Feb 18, 2015

Choose a reason for hiding this comment

ramnathv Feb 18, 2015

Choose a reason for hiding this comment

yihui Feb 19, 2015

Choose a reason for hiding this comment

ramnathv Feb 19, 2015

Choose a reason for hiding this comment

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

nsh87 commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015

timelyportfolio commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015

jcheng5 commented May 28, 2015

ramnathv commented May 28, 2015